Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where node connection map wasn't being pruned #4231

Merged
merged 2 commits into from
Apr 27, 2018
Merged

Conversation

dadgar
Copy link
Contributor

@dadgar dadgar commented Apr 27, 2018

Fixes #4230

@djenriquez
Copy link

🏎

}
}

// If we just deleted the last conn, remove it from the map
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the last connection is deleted why is numConns ==1 and not 0?


// If we just deleted the last conn, remove it from the map
if found && numConns == 1 {
delete(s.nodeConns, ctx.NodeID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do the len check and deletion right in the for loop.

@@ -40,6 +40,11 @@ func (s *Server) getNodeConn(nodeID string) (*nodeConnState, bool) {
}
}

// Shouldn't happen but rather be safe
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we dream up a reason this is possible? If not should we add a debug or trace log just in case it can help us find where an invariant is being broken.

@dadgar dadgar merged commit e44600a into master Apr 27, 2018
@dadgar dadgar deleted the b-no-node-panic branch April 27, 2018 17:38
@github-actions
Copy link

github-actions bot commented Mar 5, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants